feat: add previous period hash to proofs and roots if present #166
Conversation
lib/period.spec.js
Outdated
| block = new Block(i).addTx(Tx.deposit(i, value, ADDR, color)); | ||
| blocks.push(block); | ||
| } else { | ||
| blocks.push(new Block(i)); |
There was a problem hiding this comment.
what's the purpose to add empty blocks? 🤔
lib/period.spec.js
Outdated
| const period = new Period("'0x8b04de057fe524a3118eb7c8e14a2e55323c67fd7b6080583d1047b700b2d674'", blocks); | ||
| period.setValidatorData(slotId, ADDR); | ||
| const proof = period.proof(transfer); | ||
| expect(proof).to.eql([ |
There was a problem hiding this comment.
this test is not checking that prevHash is included in the proof, no?
How about that:
- period = new Period(prevHash, blocks)
- const proof1 = period.proof(transfer)
- period = new Period(someOtherPrevHash, blocks)
- const proof2 = period.proof(transfer)
- expect(proof1).not.equal(proof2)
There was a problem hiding this comment.
or better check for prevHash inclusion?
lib/period.spec.js
Outdated
| block = new Block(31).addTx(transfer); | ||
| blocks.push(block); | ||
|
|
||
| const period = new Period("'0x8b04de057fe524a3118eb7c8e14a2e55323c67fd7b6080583d1047b700b2d674'", blocks); |
…nd fix backwards compatibility issue.
|
Suggesting the following change for this PR: #167 |
lib/period.spec.js
Outdated
| periodAfter.usePrevPeriod(); | ||
|
|
||
| const proof = periodAfter.prevPeriodProof(); | ||
| expect(Period.verifyPrevPeriodProof(proof)).to.eql(periodAfter.periodRoot()); |
There was a problem hiding this comment.
this isn't really checking that period.periodRoot() was used in proof, no?
lib/period.js
Outdated
| } | ||
|
|
||
| // returns current period | ||
| static verifyPrevPeriodProof(proof) { |
There was a problem hiding this comment.
the name is misleading IMO. It is not verifying anything, but rather calculating period root for the proof
There was a problem hiding this comment.
Yes this is because in solidity the verify method just computes the edges if the walk and verifies they reference one another. See new commit for clarification.
* chore: rework Period object constructor 1. Set `usePrev` by default. BREAKING change 2. Accept optional third arguments of `PeriodOptions` type: allows to set validator data and set `usePrev` to false for legacy proofs. * docs: add period hashing schema
troggy
changed the title
Resolves: #165